feat(studio,core): a volume and a living meter on the group row - #3290
Conversation
a3177a2 to
ac2bbfa
Compare
5d5d596 to
291e0db
Compare
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review
Approve. Independent read at current head.
Audio graph topology is correct: output gain node between FX chain and master, with analyser tapped off the output node — honest level readings per design doc §5. Pre-allocated per-group Float32Array buffer avoids per-frame allocation. useGroupLevel throttles renders at 33ms via setTimeout without adding its own polling loop — relies on the runtime's own posting cadence.
Live/commit split mirrors FxParamRow: setLive does DOM-only write during drag, setQuiet persists with undo on release, save failure rewinds the optimistic write. STRIP_H layout constant integrates cleanly alongside TRACK_H/LANE_H.
Tests pin 7 component behaviors (slider clamping, clipping hold/decay, no-dB constraint) and 5 transport-level meter behaviors (lazy analyser creation, RMS reading, clip detection, dispose cleanup).
No issues. Ship it.
— Miga
miga-heygen
left a comment
miga-heygen
left a comment
There was a problem hiding this comment.
Review
Approve. Independent read at exact head.
Bus strip (B7) architecture is correct. The input→output gain pair creates a stable anchor point for the meter — the analyser taps output, which sits after the FX chain and before master. The groupLevel method uses RMS with a per-group reusable buffer (no per-frame allocation). Clip detection at |sample| >= 0.99 is appropriate. The "Too loud" hold timer (2s after last clip) prevents flicker.
Key correctness points:
- Analyser created lazily, one per group, on first member schedule — no wasted resources for idle groups
fftSize: 256is correct for level metering (not spectrum analysis)postGroupLevelsonly fires while playing — no idle CPU- The output gain node is correctly documented as the B5 mute splice point (before output, not after)
useGroupLevelthrottles at 33ms (~30fps) using the same pub-sub pattern asliveTime.tsTimelineGroupBusStriplive-writes on drag, commits on release — correct splitapplyGroupStripHeightscorrectly overrides only anchor rows, leaving member rows at their natural heightaudioGroupVolumeparsed once per document via thegroupInfoCacheWeakMap
Tests cover: null for unknown group, one analyser per group (lazily), RMS reading, clip flag, analyser disposal on destroy, bus strip live/commit split, level bar + clip hold, no-dB design constraint.
No issues. Ship it.
— Miga
terencecho
left a comment
terencecho
left a comment
There was a problem hiding this comment.
Review
Approve. Concurring with Miga's APPROVE at this SHA; adding independent audits below.
Cross-checks (unique)
- Analyser sizing round-trip.
analyser.fftSize = 256is set BEFORElevelBuf: new Float32Array(analyser.fftSize)reads it in the_groups.set(groupId, {...})block, so the per-group buffer is 256 samples (not the WebAudio default 2048) — matches the test mock's assertion at line 173. RMS over 256 samples at 48kHz ≈ 5.3ms window: appropriate for a level meter, not a spectrum. - Clip threshold honesty.
Math.abs(sample) >= 0.99(~-0.087 dBFS) — trips just below digital full-scale, so anything that would actually clip the master trips it. Correct semantic pin. - Playing-only gate holds.
postGroupLevels()is called insideif (clock.isPlaying())ininit.ts:2919;groupIds()early-returns if empty;.filter(null)early-returns if all groups are idle. Zero-cost when nothing is grouped or playing. - RuntimeOutboundMessage discriminant is disjoint.
type: "group-levels"is a new literal not colliding with the 10 existing message types in the union, anduseTimelinePlayer.tsmatches by bothsource === "hf-preview"ANDtype === "group-levels"before dispatch — no fall-through into thestatebranch. - Cache staleness shape.
groupInfoCache: WeakMap<Document, ...>inherits the pre-existinggroupLabelCacheshape — invalidates on iframe-reload / Document replacement, not on livedata-volumemutations. Not a regression (same shape as before); worth flagging on B5 (mute) if that path expects live-mutation reads. - Test-fix accounting. The two pre-existing
webAudioTransport.test.tsfailures were hardcoded gain-node index assertions (indices 0/2 → 0/3), correctly updated to reflect the newoutputgain node inserted between input and master. These are pin-test index shifts, not semantic-behavior changes. - CI at head: all 26 required + optional checks green after dedupe-by-name (
Preflight,regression-shards×9,Perf×5,preview-parity,Test, workflow gates).
Concur on (per Miga)
Audio graph topology (input → FX/dry → output → master, analyser tapped off output), pre-allocated Float32Array buffer, useGroupLevel 33ms throttle piggybacking runtime cadence, live/commit split mirroring FxParamRow, STRIP_H layout integration, 12 test-pin coverage across component + transport.
Ship it.
— Review by tai (pr-review)
ac2bbfa to
2f2495e
Compare
291e0db to
f8bd6d1
Compare
2f2495e to
940642b
Compare
f8bd6d1 to
ed29b75
Compare
The base branch was changed.
B7: the group bus strip — droppable, and deliberately minimal per the casual-user design constraints (groups doc §5): a volume slider, a level bar that moves with the sound, and the words "Too loud" when it clips. No dB numbers, no peak-hold readout, no routing row. Transport (core): groupInput() now routes each group through input -> [FX chain or dry passthrough] -> output -> master, with one AnalyserNode per group tapped off `output` (post-FX, so the meter reads what the bus actually outputs) — fftSize 256, level not spectrum. groupLevel(groupId) returns RMS-ish level 0..1 + a clipped flag off a reused per-group buffer (no per-frame allocation), or null when the group is idle/unknown. The runtime posts group-levels messages only while playing, piggybacking the existing message channel rather than adding a new poll loop. Studio: groupLevels.ts is a plain pub-sub store (mirrors liveTime.ts's shape) fed by useTimelinePlayer's message handler via parseGroupLevelsMessage; useGroupLevel throttles re-renders to ~33ms. TimelineGroupBusStrip renders in the group row's own `∿` lane area (STRIP_H, already sized in B2's row-height pipeline) — drag writes live via onSetAudioGroupAttributeLive, release commits one undo entry via onSetAudioGroupAttributeQuiet (packages/studio/src/hooks/ timelineAudioGroupVolume.ts, extracted from timelineTrackVisibility.ts to stay under the 600-line cap; mirrors FxParamRow's live/commit split). "Too loud" holds for ~2s after the last clipped block, tracked in the component, not the transport. volumeByGroup mirrors labelByGroup in useTimelineTrackDerivations.ts so the strip's slider round-trips the group's own data-volume. Fixed two pre-existing group-routing tests in webAudioTransport.test.ts that hardcoded gain-node creation order/count — B7 inserts an extra `output` gain node between the group's input and master (for the meter to tap), which shifted node indices the tests asserted on directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ed29b75 to
19635c0
Compare
…evel buffer non-shared
Two CI gates, both from this branch's own additions.
`File size check`: `useTimelinePlayer.ts` sat at 599 lines on main and the
group-levels branch pushed it to 605 (cap 600). Extracted the `window.message`
router — which already carried a `fallow-ignore-next-line complexity` admitting
it had outgrown its home — into `previewMessageRouter.ts`, with the fixture
lease, sender check and protocol accept-gate collapsed into one
`acceptedPreviewMessage` so the listener is a flat dispatch and the suppression
is retired rather than moved. Same branches, same refs, no behaviour change;
the file lands at 561.
`Test: runtime contract`: `levelBuf: Float32Array` resolves to
`Float32Array<ArrayBufferLike>` under `tsconfig.runtime.json`, and
`getFloatTimeDomainData` will not take a possibly-shared buffer (TS2345).
Pinned the field to `Float32Array<ArrayBuffer>`, which is what
`new Float32Array(analyser.fftSize)` already produces.
Also drops `EditorShell.selectionSync.test.tsx`'s `vi.mock("./StudioFeedbackBar")`
— main deleted that component in favour of `feedback/StudioFeedbackCard`, and
touching this file for the group prop put the dangling path in fallow's scope.
3 participants
Summary
B7 — the group bus strip: a volume slider, a level bar that moves with the sound, and the words "Too loud" when it clips. Deliberately minimal per the casual-user design constraints (groups doc §5): no dB numbers, no peak-hold readout, no routing row.
groupInput()now routes each group throughinput -> [FX chain or dry passthrough] -> output -> master, with oneAnalyserNodeper group tapped offoutput(post-FX, so the meter reads what the bus actually outputs) —fftSize256.groupLevel(groupId)returns RMS-ish level 0..1 + aclippedflag off a reused per-group buffer, ornullwhen the group is idle/unknown. The runtime postsgroup-levelsmessages only while playing.groupLevels.tsis a plain pub-sub store (mirrorsliveTime.ts's shape);useGroupLevelthrottles re-renders to ~33ms.TimelineGroupBusStriprenders in the group row's own∿lane area — drag writes live, release commits one undo entry (packages/studio/src/hooks/timelineAudioGroupVolume.ts, extracted fromtimelineTrackVisibility.tsto stay under the 600-line cap). "Too loud" holds for ~2s after the last clipped block.webAudioTransport.test.tswhose gain-node index assertions broke once B7 inserted the extraoutputgain node between a group's input and master.Depends on and stacks on #3289 (B4), #3288 (B6), #3287 (B3), #3286 (B2), #3278 (B1), #3277 (P2), #3276 (P1), #3275 (A2), #3274 (A1) — should merge after all of those.
Test plan
bun run buildcleanpackages/corefull suite: 2371/2371packages/studiofull suite: 4260/4260bunx oxfmt/bunx oxlintclean on all touched fileswebAudioTransport.test.ts(analyser lazily created/disposed per group,groupLevelnull for idle/unknown, RMS reading, clip detection),TimelineGroupBusStrip.test.tsx(Holds-line, live-drag vs release-commit, 0..2 clamp, clip/hold state machine, no-dB copy assertion)🤖 Generated with Claude Code